feat: implement filter node executor with support for deduplication, … - #85
Conversation
…comparison, and GROUPING operations
📝 WalkthroughWalkthroughThe PR adds the ChangesGroup-by filter support
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ConfigModal
participant WorkflowState
participant FilterExecutor
ConfigModal->>ConfigModal: merge node field defaults
ConfigModal->>WorkflowState: store and dispatch configuration
WorkflowState->>FilterExecutor: submit group_by with sourceKey
FilterExecutor->>WorkflowState: return grouped data and metadata
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/web/app/lib/nodeConfigs/filter.action.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. apps/web/app/workflows/[id]/components/ConfigModal.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. packages/common/src/index.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Implements a new group_by operation for the Filter node so workflows can group rows by a selected key, and updates the web UI to support additional node defaulting behavior and expose the new operation choice.
Changes:
- Added
group_byexecution path inFilterExecutor, including grouped outputs and related metadata. - Extended shared Zod validation (
FilterNodeInput) to allow the newgroup_byoperation. - Updated the web UI config modal to apply field
defaultValues when loading saved node configs, and added the new operation to the Filter node’s operation dropdown.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/nodes/src/filter/filter.executor.ts | Adds group_by executor logic and returns grouping results/metadata. |
| packages/common/src/index.ts | Extends FilterNodeInput.operation enum with group_by. |
| apps/web/app/workflows/[id]/components/ConfigModal.tsx | Applies node field defaults when loading configs; tweaks effect dependency. |
| apps/web/app/lib/nodeConfigs/filter.action.ts | Adds group_by option to the Filter operation list in the UI. |
| return { | ||
| success: true, | ||
| output: { | ||
| groupsMap: groupResult.groupMap, | ||
| groupsArray: groupResult.groupArray, | ||
| metadata: { | ||
| operation_used: operation, | ||
| total_groups: groupResult.groupArray.length, | ||
| items_processed: groupResult.total_processed, | ||
| items_without_key: groupResult.emptyCount | ||
| } | ||
| } | ||
| } |
| private handleGroupBy(sourceData: any[], sourceKey: string) { | ||
| const groupMap: Record<string, any[]> = {}; | ||
| let emptyCount = 0; | ||
| for (const item of sourceData) { | ||
| const key = this.normalizeValue(this.getValueByPath(item, sourceKey)) |
| setConfig(finalConfig); | ||
| dispatchConfig(finalConfig); | ||
|
|
||
| if (nodeConfig?.fields) { | ||
| for (const field of nodeConfig.fields) { |
| options: [ | ||
| { label: "Remove Duplicates from Provided Data(Single List)", id: "unique_rows" }, | ||
| { label: "Find New Data Only (Compare Two Lists)", id: "new_data_only" }, | ||
| { label: "Find Existing Data Only (Compare Two Lists)", id: "existing_data_only" } | ||
| { label: "Find Existing Data Only (Compare Two Lists)", id: "existing_data_only" }, | ||
| { label: "Group Data By Key", id: "group_by" }, | ||
| ], |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/nodes/src/filter/filter.executor.ts`:
- Around line 201-220: The group_by output contract is inconsistent between the
executor and node configuration. In packages/nodes/src/filter/filter.executor.ts
lines 201-220, establish the agreed group_by result shape and return its
groupsMap, groupsArray, and metadata fields; in
apps/web/app/lib/nodeConfigs/filter.action.ts lines 18-19, expose those same
fields by adding output entries or making outputSchema operation-aware. Keep
both layers aligned so grouping results are discoverable and downstream
references resolve correctly.
- Around line 119-130: Update handleGroupBy to initialize groupMap as a
prototype-free map so arbitrary normalized keys such as "__proto__",
"constructor", and "toString" are handled as ordinary groups. Preserve the
existing grouping and emptyCount behavior.
- Around line 201-206: Wrap the group_by case in the switch statement with
braces so the const groupResult declaration is scoped to that clause, while
preserving its existing logic and return behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ce523f06-9c67-446f-9ca8-2015e2a9f5c3
📒 Files selected for processing (4)
apps/web/app/lib/nodeConfigs/filter.action.tsapps/web/app/workflows/[id]/components/ConfigModal.tsxpackages/common/src/index.tspackages/nodes/src/filter/filter.executor.ts
| private handleGroupBy(sourceData: any[], sourceKey: string) { | ||
| const groupMap: Record<string, any[]> = {}; | ||
| let emptyCount = 0; | ||
| for (const item of sourceData) { | ||
| const key = this.normalizeValue(this.getValueByPath(item, sourceKey)) | ||
|
|
||
| if (key === "[EMPTY]") emptyCount++; | ||
|
|
||
| if (!groupMap[key]) { | ||
| groupMap[key] = [] | ||
| } | ||
| groupMap[key].push(item) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Use a prototype-free map for arbitrary group keys.
groupMap is initialized with {}. For a normalized key such as "__proto__", "constructor", or "toString", the lookup returns an inherited object or function. groupMap[key].push(item) then throws, and the executor returns success: false for valid input.
Proposed fix
- const groupMap: Record<string, any[]> = {};
+ const groupMap: Record<string, any[]> = Object.create(null);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private handleGroupBy(sourceData: any[], sourceKey: string) { | |
| const groupMap: Record<string, any[]> = {}; | |
| let emptyCount = 0; | |
| for (const item of sourceData) { | |
| const key = this.normalizeValue(this.getValueByPath(item, sourceKey)) | |
| if (key === "[EMPTY]") emptyCount++; | |
| if (!groupMap[key]) { | |
| groupMap[key] = [] | |
| } | |
| groupMap[key].push(item) | |
| private handleGroupBy(sourceData: any[], sourceKey: string) { | |
| const groupMap: Record<string, any[]> = Object.create(null); | |
| let emptyCount = 0; | |
| for (const item of sourceData) { | |
| const key = this.normalizeValue(this.getValueByPath(item, sourceKey)) | |
| if (key === "[EMPTY]") emptyCount++; | |
| if (!groupMap[key]) { | |
| groupMap[key] = [] | |
| } | |
| groupMap[key].push(item) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/nodes/src/filter/filter.executor.ts` around lines 119 - 130, Update
handleGroupBy to initialize groupMap as a prototype-free map so arbitrary
normalized keys such as "__proto__", "constructor", and "toString" are handled
as ordinary groups. Preserve the existing grouping and emptyCount behavior.
| case 'group_by': | ||
| if (!sourceKey) return { | ||
| success: false, | ||
| error: "sourceKey is required to group datasets" | ||
| } | ||
| const groupResult = this.handleGroupBy(normalizedSource, sourceKey); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Wrap the group_by switch clause in braces.
Biome reports lint/correctness/noSwitchDeclarations for const groupResult at Line 206. Add a block around this case so the declaration is scoped only to group_by.
Proposed fix
- case 'group_by':
+ case 'group_by': {
if (!sourceKey) return {
success: false,
error: "sourceKey is required to group datasets"
@@
}
}
}
+ }
default:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case 'group_by': | |
| if (!sourceKey) return { | |
| success: false, | |
| error: "sourceKey is required to group datasets" | |
| } | |
| const groupResult = this.handleGroupBy(normalizedSource, sourceKey); | |
| case 'group_by': { | |
| if (!sourceKey) return { | |
| success: false, | |
| error: "sourceKey is required to group datasets" | |
| } | |
| const groupResult = this.handleGroupBy(normalizedSource, sourceKey); | |
| // existing group_by case body | |
| } | |
| default: |
🧰 Tools
🪛 Biome (2.5.6)
[error] 206-206: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
(lint/correctness/noSwitchDeclarations)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/nodes/src/filter/filter.executor.ts` around lines 201 - 206, Wrap
the group_by case in the switch statement with braces so the const groupResult
declaration is scoped to that clause, while preserving its existing logic and
return behavior.
Source: Linters/SAST tools
| case 'group_by': | ||
| if (!sourceKey) return { | ||
| success: false, | ||
| error: "sourceKey is required to group datasets" | ||
| } | ||
| const groupResult = this.handleGroupBy(normalizedSource, sourceKey); | ||
|
|
||
| return { | ||
| success: true, | ||
| output: { | ||
| groupsMap: groupResult.groupMap, | ||
| groupsArray: groupResult.groupArray, | ||
| metadata: { | ||
| operation_used: operation, | ||
| total_groups: groupResult.groupArray.length, | ||
| items_processed: groupResult.total_processed, | ||
| items_without_key: groupResult.emptyCount | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Align the group_by output contract across both layers.
The executor and node configuration do not publish the same result shape. This can hide grouping results from output discovery or break downstream references.
packages/nodes/src/filter/filter.executor.ts#L201-L220: define whethergroup_byuses an operation-specific shape or the common filter shape, then return the agreed fields.apps/web/app/lib/nodeConfigs/filter.action.ts#L18-L19: add output entries forgroupsMap,groupsArray, and the new metadata fields, or makeoutputSchemaoperation-aware.
🧰 Tools
🪛 Biome (2.5.6)
[error] 206-206: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
(lint/correctness/noSwitchDeclarations)
📍 Affects 2 files
packages/nodes/src/filter/filter.executor.ts#L201-L220(this comment)apps/web/app/lib/nodeConfigs/filter.action.ts#L18-L19
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/nodes/src/filter/filter.executor.ts` around lines 201 - 220, The
group_by output contract is inconsistent between the executor and node
configuration. In packages/nodes/src/filter/filter.executor.ts lines 201-220,
establish the agreed group_by result shape and return its groupsMap,
groupsArray, and metadata fields; in
apps/web/app/lib/nodeConfigs/filter.action.ts lines 18-19, expose those same
fields by adding output entries or making outputSchema operation-aware. Keep
both layers aligned so grouping results are discoverable and downstream
references resolve correctly.
…comparison, and GROUPING operations
Summary by CodeRabbit
New Features
Bug Fixes